Skip to content

Conversation

@kevin-lii
Copy link
Contributor

@kevin-lii kevin-lii commented Mar 25, 2025

Description

Currently, CMK only allows for POST requests and this is a security concern since GET requests may leak the values of the query parameters.

Types of changes

[] Breaking change (fix or feature that would cause existing functionality to change)
[] New feature (non-breaking change which adds functionality)
[] Bug fix (non-breaking change which fixes an issue)
[] Enhancement (improves an existing feature and functionality)
[] Cleanup (Code refactoring and cleanup, that may add test cases)
[] build/CI
[] test (unit or integration test code)
[X] Feature/Enhancement Scale

[] Major
[X] Minor

How Has This Been Tested?

This was tested by manually building CMK and using a proxy to confirm that POST requests are being sent by CMK when the configuration value is set.

Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks okay, a couple of comments added

@rohityadavcloud rohityadavcloud added this to the 6.5.0 milestone Mar 27, 2025
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevin-lii build is failing. Can you please check?

@kevin-lii kevin-lii requested a review from shwstppr April 8, 2025 17:44
@kevin-lii
Copy link
Contributor Author

Sorry about the delay but should fix the issues with this PR

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code lgtm

Copy link

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

On Existing release of cmk

  1. Enabled the global setting "enforce.post.requests.and.timestamps” to true

Getting exception

(test2) 🐱 > create volume name=testvolume4 diskofferingid=6eba93c8-bfc5-45fb-8620-e108b71d1f4a zoneid=932cfb3e-be7e-4d6f-8730-3b0f520c58bf

🙈 Error: (HTTP 400, error code 9999) State changing command createVolume needs to be sent using POST request


After building the cmk based on the pr

  1. Build the new cmk after cloning the repo

  2. Able to execute the CMK calls without any issue

Request Method POST

@rohityadavcloud rohityadavcloud merged commit a6552ee into apache:main Jul 14, 2025
1 check passed
shwstppr added a commit to shwstppr/cloudstack-cloudmonkey that referenced this pull request Aug 26, 2025
PR apache#161 introduced `postrequest` config but there was no way to update that config other than updating the config file.
This PR fixes the behaviour. Also adds a log to highlight what HTTP
method is used.

Signed-off-by: Abhishek Kumar <[email protected]>
shwstppr added a commit that referenced this pull request Aug 26, 2025
PR #161 introduced `postrequest` config but there was no way to update that config other than updating the config file.
This PR fixes the behaviour. Also adds a log to highlight what HTTP method is used.

Signed-off-by: Abhishek Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants